Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

nixos/systemd-repart: enable creating root partition #232533

Merged
merged 1 commit into from
May 22, 2023
Merged

nixos/systemd-repart: enable creating root partition #232533

merged 1 commit into from
May 22, 2023
+118 −34

Conversation

nikstur
Copy link
Contributor

@nikstur nikstur commented May 18, 2023

Description of changes

Add the ability to create the root partition while in the initrd. This enables to build system image with only a Nix store where the root partition is dynamically created.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@nikstur nikstur requested a review from a team as a code owner May 18, 2023 01:24
@nikstur nikstur requested a review from ElvishJerricco May 18, 2023 01:25
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: systemd 8.has: module (update) This PR changes an existing module in `nixos/` labels May 18, 2023
@nikstur nikstur requested review from flokli and arianvp May 18, 2023 01:25
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels May 18, 2023
ElvishJerricco
ElvishJerricco reviewed May 18, 2023
View reviewed changes
Copy link
Contributor

@ElvishJerricco ElvishJerricco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concerns are the escapeSystemdPath thing and another thing I just realized. We definitely don't want it looking at the config files in /sysroot/etc/repart.d before activation because those could be out of date. So the after = ["sysroot.mount"]; should almost certainly be after = ["nixos-activation.service"]; That should also be added to requires.

Also, FYI, it's generally a little more convenient / appropriate to use systemd.services.foo.after rather than systemd.services.foo.unitConfig.After (same goes for requires). This makes it easier to add to with the nixos module system. Not a big deal though.

@nikstur nikstur force-pushed the systemd-repart-create-root branch from 6950965 to da205e6 Compare May 18, 2023 22:28
@nikstur
Copy link
Contributor Author

nikstur commented May 18, 2023 •
edited
Loading

Also, FYI, it's generally a little more convenient / appropriate to use systemd.services.foo.after rather than systemd.services.foo.unitConfig.After (same goes for requires). This makes it easier to add to with the nixos module system. Not a big deal though.

Good to know, changed it accordingly.

We definitely don't want it looking at the config files in /sysroot/etc/repart.d before activation because those could be out of date.

This shouldn't be an issue in the initrd since the config files are now baked into the initrd and we tell systemd-repart explicitly to look for the configs in the initrd with --definitions. It will not try to look for them elsewhere.

However in userspace, this might be an issue. I added the ordering there.

@nikstur nikstur force-pushed the systemd-repart-create-root branch from da205e6 to ef80e11 Compare May 19, 2023 13:55
@nikstur nikstur requested review from ElvishJerricco and arianvp May 19, 2023 13:55
@nikstur
Copy link
Contributor Author

nikstur commented May 22, 2023

@arianvp @ElvishJerricco is there anything else to be done? Or is this good to go?

@ElvishJerricco ElvishJerricco merged commit 636e03b into NixOS:master May 22, 2023
@ElvishJerricco
Copy link
Contributor

Thanks!

@arianvp arianvp mentioned this pull request May 23, 2023
Closed
@ElvishJerricco ElvishJerricco mentioned this pull request May 29, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arianvp arianvp arianvp approved these changes

@flokli flokli Awaiting requested review from flokli

@ElvishJerricco ElvishJerricco Awaiting requested review from ElvishJerricco

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: systemd 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nikstur @ElvishJerricco @arianvp